-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix missing in.current_cell parameter entry. #6240
base: main
Are you sure you want to change the base?
Fix missing in.current_cell parameter entry. #6240
Conversation
Oke, it looks like this is ready for review. @danieldouglas92 will add a test based on his test setup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good, can you rebase to fix the documentation tester and update the test results?
Also add a changelog entry if this was introduced before 3.0 (which I think it was), just so we know it is already fixed if the issue comes up for anyone.
634ed1c
to
a7ce884
Compare
a7ce884
to
9e6e137
Compare
I have rebased the pull request and fixed the test results and made a changelog entry. So ready for another review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good to go if you address my comments and squash your commits.
@@ -0,0 +1,5 @@ | |||
Fixed: Adds current cell to material model intpust |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed: Adds current cell to material model intpust | |
Fixed: Add the current cell to the material model inputs |
@@ -0,0 +1,5 @@ | |||
Fixed: Adds current cell to material model intpust | |||
in the cpo particle property to fix the Olivine: Karato | |||
2008 mineral type. Also added a test to test this feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2008 mineral type. Also added a test to test this feature. | |
2008 mineral type. Also added a test for this feature. |
in.current_cell was never filled in the material_model_inputs struct, but when a cpo particle has
Minerals = Olivine: Karato 2008
it is used, leading to the crasch in #6237.This fixes #6237 for me. @danieldouglas92 could you give this a go?